Move sponsors to DB #9512

berinhard · 2021-05-11T14:46:26Z

@ewdurbin and @dstufft I'm opening this draft PR so we can async talk about the progress of #9503.

Right now this PR introduces:

The new Sponsor model;
A command to load the sponsors into the database;

I'll add inline comments with specific questions I have.

warehouse/cli/sponsors.py

berinhard · 2021-05-11T14:52:18Z

warehouse/cli/sponsors.py

+        params["color_logo_url"] = img
+        params["white_logo_url"] = f"white/{img}"


@ewdurbin as you've comment, I'm adding the URLs directly, but I don't think we can totally rely on current URLs in Pypi. I mean, take a look at current Google's logo URL:

https://pypi.org/static/images/sponsors/color/google.62f52fb9.png

It has this hash 62f52fb9 suffix that I'm assuming it changes every deploy. So, how can we define the URLs here?

Also, what do you think about the directory structure I proposed: having the colored logo in a directory and all the white logos should be placed in the white children directory with the same file name.

That hash is stable as long as pypi-theme doesn't change (and it won't since we broke the hosted private index when we turned off support for non SNI clients 😂 😬 😭 )

I don't have a good amount of time to review in depth, but I'm not sure if a directory structure is strictly necessary.

Unless there's a good reason for it, I would presume we'll just take whatever image is uploaded, discard the name, and toss it in a static assets bucket under sponsor-logos/{UUID}.png

warehouse/sponsors/models.py

I added #noqa for a few of the activity strings from the sponsors.

I'm used to use direnv to configure env variables. I had to create it to always have WAREHOUSE_IPYTHON_SHELL enabled.

berinhard · 2021-05-14T14:56:00Z

Hey everyone, I did some progress with this!

As you can see, I changed the /sponsors/ page to read the sponsors from the database by replacing the existing template route to a custom view. Although this is the simplest solution I could think and it works for that page, I'm afraid this is not true for the sidebar or footer logos.

From what I've saw in the code, I feel that the csi template tag is a good candidate to go with. But, this will also introduce a conceptual change: now both sidebar and footer will require JS code to display the sponsors. Is this a problem?

I don't think it'll be, but I fell more comfortable asking before coding since I'm new in the project.

(please, do not consider the failing tests because I'm still under seeking for solutions mode. I'll work on them once I know my suggestions are valid alternatives)

ewdurbin · 2021-05-17T15:39:37Z

As you can see, I changed the /sponsors/ page to read the sponsors from the database by replacing the existing template route to a custom view. Although this is the simplest solution I could think and it works for that page, I'm afraid this is not true for the sidebar or footer logos.

From what I've saw in the code, I feel that the csi template tag is a good candidate to go with. But, this will also introduce a conceptual change: now both sidebar and footer will require JS code to display the sponsors. Is this a problem?

I think that the CSI approach has merit, and shows significant consideration on your part to note the additional query necessary on every request to render sponsor information throughout the site. But I think the query for fetching sponsor name, description, inclusion areas (footer/sidebar), and logo url should be fairly lightweight and doable in a single sub millisecond query for each request. Given that, I the caveat of requiring JS to show our sponsor information might be worth making the trade off of adding a new query to every request.

It's also more simple :)

warehouse/sponsors/models.py

warehouse/templates/pages/sponsors.html

This reverts commit 8d5b1cc.

This reverts commit 582581f.

…lude" This reverts commit c5f0d7a.

ewdurbin · 2021-06-01T15:46:43Z

Note: We should ensure that we sanitize (for JavaScript an any esoteric tags/properties) any strings submitted via Admin before displaying on PyPI.org. This will ensure that even a hijacked Admin/Moderator/Sponsor Coordinator account won't result in an XSS for PyPI.

warehouse/sponsors/models.py

warehouse/migrations/versions/590c513f1c74_new_psf_staff_boolean_flag.py

ewdurbin

We reviewed this together on a call to finish our the last few details and get it shippable.

There is one remaining task to enable image uploads that will be handled in a follow-on Pull Request.

Thank you for the thorough work here @berinhard!

di

This looks solid! Nice work!

di · 2021-06-07T23:29:41Z

Makefile

 	xz -d -f -k dev/$(DB).sql.xz --stdout | docker-compose run --rm web psql -h db -d warehouse -U postgres -v ON_ERROR_STOP=1 -1 -f -
 	docker-compose run --rm web python -m warehouse db upgrade head
 	$(MAKE) reindex
+	docker-compose run web python -m warehouse sponsors populate-db


Do we have a plan to run this in production?

As an aside, this is a lot of work for something we only need once. Wow!

Yes, this will be run then axed.

di · 2021-06-07T23:33:38Z

warehouse/accounts/__init__.py

+
+    # user must have base admin access if any admin permission
+    if principals:
+        principals.append("group:with_admin_access")


This name confused me a bit because of our existing "admins" group, maybe something like:

Suggested change

principals.append("group:with_admin_access")

principals.append("group:with_admin_dashboard_access")

?

Makes sense! Fixed in 384278a

berinhard · 2021-06-08T15:51:28Z

Not sure why, but my last build failed but I'm not sure how to fix it. I'm still confused with the dependency management tools we're using. I tried to update the main.txt file using the make command explained in the docs, but the output says it's up to date:

$ make requirements/main.txt
make: 'requirements/main.txt' is up to date.

The error from CI is:

ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
Collecting google-api-core<2.0.0dev,>=1.21.0
    google-api-core<2.0.0dev,>=1.21.0 from https://files.pythonhosted.org/packages/f6/72/6dc3c3c4576fedc409fd825bf71e22d0f448bd68143c79cdbd6216744282/google_api_core-1.30.0-py2.py3-none-any.whl#sha256=92cd9e9f366e84bfcf2524e34d2dc244906c645e731962617ba620da1620a1e0 (from google-cloud-core==1.6.0->-r requirements/main.txt (line 280))
Error: Process completed with exit code 1.

But google-api-core do have 2 hash entries in requirements/main.txt and this branch is up to date the main one after @ewdurbin commit 96d9d4f.

I'm not sure what went wrong =S

ewdurbin · 2021-06-08T15:51:45Z

dang, looks like we're hitting the google api dependency breakage again 😵

berinhard · 2021-06-08T15:56:38Z

Not sure if this is related, but they've released a new package version 1 hour ago: https://pypi.org/project/google-api-core/#history

di · 2021-06-08T16:00:10Z

Technically not google-api-core's fault, just how pypa/pip#9644 manifests here.

The solution is to upgrade to the latest version.

berinhard · 2021-06-08T16:16:04Z

Thanks @di! I hope I managed to fix this on #9636

ewdurbin · 2021-06-08T16:17:07Z

Right, not blaming the app itself, to rephrase, once again struck by pypa/pip#9644, which is triggered by google-api dependency. (our only with extras)

ewdurbin · 2021-06-08T16:18:45Z

aight! let's rebase and ship this!

The underlying datatype for URLType is `Text` and we don't use any of the functionality from the type at all. Introduced in pypi#9512 We don't need to carry the library any further. Signed-off-by: Mike Fiedler <[email protected]>

berinhard added 4 commits May 11, 2021 11:14

Create Sponsor model

9e2a8aa

Service column can be empty

8e6fab6

Command to load sponsors to database

3b49e92

initdb should load sponsors

e2d039c

berinhard commented May 11, 2021

View reviewed changes

warehouse/cli/sponsors.py Show resolved Hide resolved

berinhard commented May 11, 2021

View reviewed changes

warehouse/sponsors/models.py Outdated Show resolved Hide resolved

berinhard added 4 commits May 12, 2021 14:06

Add sqlalchemy-utils as a dependency

02d450a

Use URL type as fields and refactor to have a single migration

5b5f150

Populate images URL using current pypi static as the URL prefix

36ffefd

Fix code lint warnings

b6bf1de

I added #noqa for a few of the activity strings from the sponsors.

ewjoachim mentioned this pull request May 13, 2021

Define banners in the DB #9525

Closed

berinhard added 2 commits May 14, 2021 11:43

Add direnv's .envrc to gitignore

918203a

I'm used to use direnv to configure env variables. I had to create it to always have WAREHOUSE_IPYTHON_SHELL enabled.

Change sponsors page to read data from db instead of HTML include

c5f0d7a

berinhard added 2 commits May 14, 2021 12:44

Fix routes test

582581f

Unit test sponsors page

8d5b1cc

ewdurbin reviewed May 17, 2021

View reviewed changes

berinhard added 11 commits May 18, 2021 10:27

Better name for url field

5f838e4

Replace pickle by pg's array type

023472f

Update migration file

f8247e8

Revert "Unit test sponsors page"

6d351c3

This reverts commit 8d5b1cc.

Revert "Fix routes test"

c761787

This reverts commit 582581f.

Revert "Change sponsors page to read data from db instead of HTML inc…

4b92c8f

…lude" This reverts commit c5f0d7a.

Test utils for sponsors app

77b8d47

Add sponsors list to all requests (except for static files)

9f9aec0

Update templates to read sponsors from request variable

a70dc28

Reformat

0581a0f

Camoify logos

28dee11

Add simple unit test to make explicit validation done by readme lib

ed003db

berinhard commented Jun 3, 2021

View reviewed changes

warehouse/sponsors/models.py Show resolved Hide resolved

berinhard mentioned this pull request Jun 3, 2021

Migrate all Sponsor data to the database. #9503

Closed

Merge branch 'main' into feature/sponsors-db

96d9d4f

berinhard marked this pull request as ready for review June 7, 2021 11:39

berinhard added 4 commits June 7, 2021 11:16

Add sponsors app to the docs

69f929c

Add new boolean flag is_psf_staff to user model

dd25985

Introduce PSF Staff group and permission

e1d9e20

Configure permissions considering PSF staff members

3c13356

ewdurbin reviewed Jun 7, 2021

View reviewed changes

warehouse/migrations/versions/590c513f1c74_new_psf_staff_boolean_flag.py Outdated Show resolved Hide resolved

berinhard added 2 commits June 7, 2021 12:16

Use SQL false function instead

36d957a

Refactor permissions introducing a base one to guarantee admin access

6bae6c3

ewdurbin requested a review from di June 7, 2021 19:51

ewdurbin approved these changes Jun 7, 2021

View reviewed changes

di approved these changes Jun 7, 2021

View reviewed changes

Rename admin dashboard access permission

384278a

Merge branch 'main' into feature/sponsors-db

529f0f1

ewdurbin merged commit 51410fe into pypi:main Jun 8, 2021

ewdurbin mentioned this pull request Jun 16, 2021

Sponsor logos are not bounded by CSS #9676

Closed

ewdurbin mentioned this pull request Jul 14, 2021

Add Bloomberg footer image #9454

Closed

miketheman mentioned this pull request Jun 9, 2023

refactor: replace URLType with Text #13899

Merged

		params["color_logo_url"] = img
		params["white_logo_url"] = f"white/{img}"

	principals.append("group:with_admin_access")
	principals.append("group:with_admin_dashboard_access")

Move sponsors to DB #9512

Move sponsors to DB #9512

Uh oh!

Conversation

berinhard commented May 11, 2021

Uh oh!

Uh oh!

berinhard May 11, 2021

Choose a reason for hiding this comment

Uh oh!

ewdurbin May 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ewdurbin May 11, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

berinhard commented May 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ewdurbin commented May 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ewdurbin commented Jun 1, 2021

Uh oh!

Uh oh!

Uh oh!

ewdurbin left a comment

Choose a reason for hiding this comment

Uh oh!

di left a comment

Choose a reason for hiding this comment

Uh oh!

di Jun 7, 2021

Choose a reason for hiding this comment

Uh oh!

ewdurbin Jun 8, 2021

Choose a reason for hiding this comment

Uh oh!

di Jun 7, 2021

Choose a reason for hiding this comment

Uh oh!

berinhard Jun 8, 2021

Choose a reason for hiding this comment

Uh oh!

berinhard commented Jun 8, 2021

Uh oh!

ewdurbin commented Jun 8, 2021

Uh oh!

berinhard commented Jun 8, 2021

Uh oh!

di commented Jun 8, 2021

Uh oh!

berinhard commented Jun 8, 2021

Uh oh!

ewdurbin commented Jun 8, 2021

Uh oh!

ewdurbin commented Jun 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ewdurbin May 11, 2021 •

edited

Loading

berinhard commented May 14, 2021 •

edited

Loading

ewdurbin commented May 17, 2021 •

edited

Loading